Skip to content

Conversation

@zachschuermann
Copy link
Collaborator

@zachschuermann zachschuermann commented Apr 9, 2025

What changes are proposed in this pull request?

Rename ScanData to ScanMetadata and Scan::scan_data to Scan::scan_metadata (and corresponding FFI). Additionally, renames TableChangesScanData to TableChangesScanMetadata. Additional docs/refactor coming in #768

This PR affects the following public APIs

breaking changes:

  1. rename ScanData to ScanMetadata
  2. rename Scan::scan_data() to Scan::scan_metadata()
  3. (ffi) rename free_kernel_scan_data() to free_scan_metadata_iter()
  4. (ffi) rename kernel_scan_data_next() to scan_metadata_next()
  5. (ffi) rename visit_scan_data() to visit_scan_metadata()
  6. (ffi) rename kernel_scan_data_init() to scan_metadata_iter_init()
  7. (ffi) rename KernelScanDataIterator to ScanMetadataIterator
  8. (ffi) rename SharedScanDataIterator to SharedScanMetadataIterator

How was this change tested?

existing

resolves #816

@codecov
Copy link

codecov bot commented Apr 9, 2025

Codecov Report

Attention: Patch coverage is 70.37037% with 24 lines in your changes missing coverage. Please review.

Project coverage is 84.89%. Comparing base (8961e97) to head (a5269fb).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
ffi/src/scan.rs 0.00% 16 Missing ⚠️
kernel/src/scan/mod.rs 80.00% 0 Missing and 3 partials ⚠️
kernel/src/table_changes/scan_file.rs 78.57% 0 Missing and 3 partials ⚠️
kernel/src/table_changes/log_replay.rs 80.00% 0 Missing and 1 partial ⚠️
kernel/src/table_changes/scan.rs 75.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #817      +/-   ##
==========================================
+ Coverage   84.87%   84.89%   +0.02%     
==========================================
  Files          83       83              
  Lines       20311    20316       +5     
  Branches    20311    20316       +5     
==========================================
+ Hits        17239    17248       +9     
+ Misses       2221     2219       -2     
+ Partials      851      849       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@zachschuermann zachschuermann changed the title refactor!: rename ScanData to ScanFiles refactor!: rename ScanData to ScanMetadata Apr 9, 2025
@github-actions github-actions bot added the breaking-change Change that require a major version bump label Apr 9, 2025
Copy link
Member

@nicklan nicklan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice. Just a few more places where we call this "scan data":

kernel/tests/golden_tables.rs
276:    let mut scan_metadata = scan.scan_metadata(&engine).expect("scan data");
292:    let scan_metadata: Vec<_> = scan.scan_metadata(&engine).expect("scan data").collect();

kernel/src/table_changes/scan_file.rs
81:/// for res in scan_metadata { // scan data table_changes_scan.scan_metadata()

kernel/src/table_changes/scan.rs
183:    /// deletion vectors present in the commit. The engine data in each scan data is guaranteed

ffi/src/scan.rs
178:/// Call the provided `engine_visitor` on the next scan data item. The visitor will be provided with

kernel/src/table_changes/log_replay.rs
30:/// Scan data for a Change Data Feed query. This holds metadata that is needed to read data rows.

ffi/examples/read-table/read_table.c
86:// For each chunk of scan data (which may contain multiple files to scan), kernel will call this
297:    print_error("Failed to construct scan data iterator.", (Error*)data_iter_res.err);
304:  print_diag("\nIterating scan data\n");
311:      print_error("Failed to iterate scan data.", (Error*)ok_res.err);
315:      print_diag("Scan data iterator done\n");

kernel/src/scan/state.rs
155:/// for res in scan_metadata { // scan data from scan.scan_metadata()

Copy link
Collaborator

@sebastiantia sebastiantia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, found some neglected architecture doc that i had no idea existed

ScanFileIterator --> ScanDataIterator

@zachschuermann zachschuermann merged commit 10bdee7 into delta-io:main Apr 9, 2025
20 of 21 checks passed
@zachschuermann zachschuermann deleted the scan-data-rename branch April 9, 2025 23:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking-change Change that require a major version bump

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Rename ScanData to ScanFiles for clarity

4 participants